-
Notifications
You must be signed in to change notification settings - Fork 229
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
KTX read-only library #324
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer if the additional library was only built when an option is set to build the read-only library. However it should always be built by CI so CI should set the option.
…ting functions in read-only library ktx_read.
…ays ON on iOS and emscripten).
The tools @MarkCallow Would you approve that? |
CI should certainly build Do you think we should include the read only library in the installers? Even if we do, these 2 tools should be linked against the full |
I've changed my mind about not having |
My thought was saving memory by using the smaller ktx_read, but in that case this does not make sense. |
…figuration yields error if type is invalid for current platform
Alright, I quickly tested if a read-only WebAssembly variant makes sense, but the current version already offers no write capability (thus it's already very small). |
I do expect to add write capabilities to the WebAssembly build in the future so people can write web tools to encode files. Please check that the Emscripten build disables writing in the same way as ktx_read, i.e. using the same compile options. However do not change the name of the Emscripten library at this time as I do not want to confuse existing users. We'll need to provide warning of the change. |
…ad` (instead of `ktx`). Those are read-only and this ensures it. If creating KTX is required, we'd need to make additional targets and link against `ktx` again.
I now link Once write functionality in web is required, we should create additional targets and (optionally) set the |
I cannot find any compile option being set to omit write from the Emscripten build. I think this functionality is missing from the .wasm module just because the wrapper does not have any write functions so they're never linked from libktx. Basically an example of why a static read-only library is not needed. When we make a .wasm module that supports write, at that time we must use the same ktx_read mechanisms in CMakeLists.txt to enable building of a read-only wrapper. For now I think this PR is good to go. |
* feat: ktx_read, an experimental read-only library * feat: Allow forcing libraries to be static. Useful for flexible integration.
* feat: ktx_read, an experimental read-only library * feat: Allow forcing libraries to be static. Useful for flexible integration.
* feat: ktx_read, an experimental read-only library * feat: Allow forcing libraries to be static. Useful for flexible integration.
* feat: ktx_read, an experimental read-only library * feat: Allow forcing libraries to be static. Useful for flexible integration.
Added an additional library target
ktx_read
for users who only want to read KTX files. Especially useful if binary size is a concern.At the moment this additional lib is built by default. We could make options to en-/disable it (also for the default lib).
Thanks!